Skip to content

Support type checking with TY - #8441

Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 74 commits into
microsoft:mainfrom
jenshnielsen:ty_0_73_support_1
Draft

Support type checking with TY#8441
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 74 commits into
microsoft:mainfrom
jenshnielsen:ty_0_73_support_1

Conversation

@jenshnielsen

Copy link
Copy Markdown
Collaborator

WIP pr. Will be broken up to review in smaller bits

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.63492% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.17%. Comparing base (f33de80) to head (c4afb30).

Files with missing lines Patch % Lines
src/qcodes/dataset/legacy_import.py 56.00% 11 Missing ⚠️
src/qcodes/dataset/data_set.py 0.00% 7 Missing ⚠️
...rc/qcodes/instrument_drivers/Keysight/Infiniium.py 12.50% 7 Missing ⚠️
...codes/instrument_drivers/Keithley/Keithley_7510.py 0.00% 3 Missing ⚠️
src/qcodes/parameters/parameter.py 87.50% 3 Missing ⚠️
src/qcodes/dataset/json_exporter.py 0.00% 2 Missing ⚠️
src/qcodes/dataset/data_set_in_memory.py 66.66% 1 Missing ⚠️
src/qcodes/instrument/ip_to_visa.py 0.00% 1 Missing ⚠️
...codes/instrument_drivers/AlazarTech/dll_wrapper.py 0.00% 1 Missing ⚠️
...ivers/QuantumDesign/DynaCoolPPMS/private/server.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8441      +/-   ##
==========================================
+ Coverage   71.15%   71.17%   +0.01%     
==========================================
  Files         305      305              
  Lines       31976    32014      +38     
==========================================
+ Hits        22753    22785      +32     
- Misses       9223     9229       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Scope ty to src and tests and exclude the legacy Decadac driver, mirroring the existing pyright config. Disable import resolution rules for the drivers that depend on optional packages, as already done for mypy. Check against all platforms so that Windows only drivers are type checked independently of the platform ty runs on.
Parameter used to replace its own get_raw/set_raw methods with the
implementation generated from get_cmd/set_cmd. Assigning over a method
makes type checkers infer get_raw/set_raw to be instance attributes of
Parameter, which made every subclass implementing them as regular
methods an invalid override.

Store the generated implementation on the instance and let get_raw and
set_raw dispatch to it. They stay marked abstract so that
_implements_get_raw keeps reporting False for Parameter itself.

Clears 66 ty diagnostics.
add_parameter always binds the new parameter to self, so defaulting
TParameter to a bare Parameter, which expands to
Parameter[Any, InstrumentBase | None], wrongly claimed the instrument
was InstrumentBase | None. As InstrumentTypeVar_co is covariant this
made the result unassignable to the Parameter[SomeType, Self]
annotations drivers use.

ty applies a PEP 696 typevar default before considering the return type
context, so it hit the default rather than solving from the declared
type. mypy and pyright were unaffected.

Clears 33 ty diagnostics.
store_array_to_database asserted that the measured array has an
array_id, but passed the array_id of its setpoint arrays straight to
add_result without checking them. Those are different arrays, so a
legacy dataset with an unnamed setpoint array failed deep inside the
data saver. Raise a clear ValueError instead.

Hoist the setpoint arrays and their ids out of the loops rather than
re-indexing set_arrays on every iteration, and drop a pyright
suppression that the qcodes_loop annotations make unnecessary.
self.parameter was a lambda with name, full_name, label and unit
attached to it. A small dataclass expresses that directly and drops
seven type checker suppressions.

It is still marked as a hack: CombinedParameter does not inherit from
Parameter or ParameterBase, so it has to fake the parts of their api
that it is expected to provide. The object stays callable, returning
None as the lambda did, in case external code relies on it.

The units deprecation warning now runs before the object is built, which
is safe because the class has no custom __repr__.
ty does not recognise a TypedDict that is generic over more than one
type variable as a mapping when one of those type variables has a PEP
696 default, so it rejects re-expanding the kwargs with **. A TypedDict
generic over a single such type variable is accepted, so this is a bug
rather than something the code should work around.

Suppress it at the five subclasses that forward their kwargs on, and
document the reason once on ParameterBaseKWArgs.
Further reduction showed the trigger is not a TypedDict being generic
over more than one type variable. One type parameter with any non-Any
PEP 696 default is enough, and the problem is not specific to **
expansion: ty computes the upper bound of the synthesized Self as the
default specialization, so every other specialization is rejected by the
members that bind Self.
The driver narrowed root_instrument and instrument to the concrete
driver classes with an annotation plus a suppression, and worked around
pyvisa typing the return of read_binary_values and query_binary_values
as Sequence[float] regardless of the requested container. Spell both as
cast instead, which all three type checkers accept and which drops five
suppressions.
_finalize_res_dict_standalones built intermediate lists whose element
type was inferred from the branch that built them rather than from the
declaration. dict is invariant in its value type, so a list of
dict[str, str] is not assignable to a list of dict[str, VALUE].

Append and extend directly instead, which gives the dict literals the
declared element type as context. Note that spelling this as
res_list += [...] is not enough, pyright does not propagate the element
type through the augmented assignment.
_check_error_code read __name__ off a Callable, which the type system
does not guarantee. Annotating the parameter more precisely would risk
breaking the assignment to c_func.errcheck, since the parameter is
contravariant against ctypes own typing, so fall back to repr instead.
This also keeps the log line useful if errcheck is ever handed something
that is not a function.
set_colorbar_extend deliberately writes to a private matplotlib
attribute, as the surrounding docstring explains, because Colorbar has
no setter for extend. Extend the existing mypy suppression to ty.
The decorator tags the decorated function with a marker attribute that
ParameterBase later reads. A Callable has no such attribute as far as
the type system is concerned, so extend the existing mypy suppression to
ty.
Without an annotation ty infers the value type of the dictionary as
Any | None | tuple[str, int], picking up the None from the later
pop(sock, None), which then makes indexing the address tuple an error.
Declare the intended type instead.
dict.fromkeys with no value is typed as dict[str, Any | None], so every
read of the processed data had to be suppressed. The loop below assigns
every key anyway, so start from an empty dict with the intended type and
drop the two suppressions.
numpy_ints and numpy_floats were tuples of bare type, so the element
type carried no information and registering sqlite adapters for them
could not be checked.

Narrowing them surfaced that _adapt_float only declared float, even
though it is registered for the numpy float types as well. Annotate it
like _adapt_complex next to it, which already accepts its numpy
counterpart. The two changes are in one commit because the adapter
signature is only wrong once the tuples are narrowed.
ParamSpec._from_dict narrows the parameter to ParamSpecDict, which
carries the extra depends_on and inferred_from fields that the base
ParamSpecBaseDict does not. That is a deliberate Liskov violation which
already carried a mypy suppression, so extend it to ty.
IPToVisa deliberately injects VisaInstrument ahead of IPInstrument in
the MRO so that an IPInstrument can be driven by the pyvisa-sim backend,
as the class docstring explains. The two bases declare set_address
incompatibly, which already carried a mypy suppression, so extend it to
ty.
The Alazar boards report a CPLD version as an int, so get_idn widens the
value type of the returned dict. The existing TODO records that this is
inconsistent with the base class, and the override already carried a
mypy suppression, so extend it to ty.
The override named its parameter name while
DelegateAttributes.__getattr__ names it key, so the two differ for a
caller passing it by keyword. Python only ever calls __getattr__
positionally, so this is a real but harmless Liskov violation and is
simpler to fix than to suppress.
The colorbar returned for a 1D plot is None, so the entry taken from the
returned list has to be checked before its label is set. Doing that with
an assert also documents that the entries are optional.

Saving used Axes.figure, which matplotlib types as Figure or SubFigure,
and a SubFigure has no savefig. Ask for the root figure instead.
snapshot_raw is documented as the way to get the snapshot of a run as a
JSON string, and the snapshot notebooks use it, but it was declared only
on DataSet. DataSetInMem carried the same data under the private
_snapshot_raw, and the protocol declared only that, so reading it from
the dataset a measurement hands back did not type check.

Declare it on the protocol and add the public property to DataSetInMem,
mirroring DataSet. This also removes the suppression that
test_snapshot.py needed for exactly this, along with its comment saying
the property is not part of the protocol.
A run only has a snapshot if one was recorded, so snapshot and
snapshot_raw are both optional. The notebook indexed and passed them on
without checking.

Assert once where each is first read, which also tells the reader they
are optional, and reuse the already checked value in the diff at the end
rather than reading it from the dataset again.
A MultiParameter that measures more than one array returns a tuple of
arrays. DataSaver.add_result has always unpacked such results, but
ValuesType had no arm for them, so type checkers rejected the call.
Add ty to the test extra and run it next to mypy and pyright. ty
understands Jupyter notebooks so this covers the example notebooks in
docs, which the other type checkers do not look at. The docs extra is
now installed in that job since the notebooks import scipy, and the
interpreter is passed explicitly because ty only auto discovers virtual
environments.
ty does not understand the error codes in a mypy type: ignore comment,
so every deliberately wrong call in the test suite is reported twice.
Add a matching ty: ignore comment on those lines. This is the
mechanical part of getting the tests to type check with ty and leaves
only the diagnostics that are not already suppressed for mypy.
These are calls that deliberately pass invalid arguments to check that
they are rejected at runtime. They are only reported by pyright and ty
since the enclosing test functions are untyped and therefore skipped by
mypy.
The tuple only contains numpy types but was annotated as also containing
the builtin complex. Since complex in an annotation implicitly means
int or float or complex, that made calls such as complex_type(1 + 2j)
be checked against int and float too.
Without an annotation ty infers the type of the module level constant
from its default value, so reassigning WEBSOCKET_PORT to select another
port, as the test suite does, is an error.
The element type of an unannotated dict or list literal is inferred from
its content, and containers are invariant, so a literal such as
{name: (11, 11)} is not a dict[str, tuple[int, ...]]. Declare the type
that the receiving function expects instead.
Parameter.step and DelegateParameter.source are optional and are read
back through a property, so a type checker cannot know that the value
assigned earlier in the test is still there.
The deprecation shim tests create TypeVar objects to hand to
_make_deprecated_typevars_getattr and read a TypeVar that only the
module level __getattr__ provides. Neither is something a type checker
can follow, and mypy and pyright accept both without complaint.
One instrument in this test module deliberately assigns a parameter over
a method, which is an error by design and now says so. The blanket
ignore on the instrument that overrides a property is no longer needed
by any of the three type checkers.
The list is later filled with two dimensional arrays, which does not fit
the one dimensional element type inferred from the initial content now
that numpy arrays carry their shape in the type.
The wrapped callable is a function in practice but its declared type
does not guarantee that. mypy and pyright both accept the attribute
access.
The test suite is now free of ty diagnostics, so there is no longer a
reason to limit the CI run to src and docs. The paths to check are
configured in pyproject.toml.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant